ci: harden GitHub Actions against supply chain attacks#1722
Conversation
- Pin all third-party actions to immutable commit SHAs - Add top-level permissions: contents: read to all workflows - Add StepSecurity Harden Runner (egress-policy: audit) to every job - Add SocketDev/action (firewall-free) + sfw install wrapper to tron-smart-contracts jobs - Pin github/codeql-action/upload-sarif to SHA (runs with security-events: write) Closes RequestNetwork/private-issues#282
Greptile SummaryThis PR hardens seven GitHub Actions workflows against supply chain attacks by pinning all third-party actions to immutable commit SHAs, adding StepSecurity Harden Runner to every job, applying the SocketDev firewall to package install steps, and migrating
Confidence Score: 4/5Safe to merge for most jobs, but three workflows still forward PATs/tokens to externally-owned reusable workflows referenced by a mutable branch tag. The bulk of the hardening is solid — direct action pins, Harden Runner, SocketDev firewall, and the script-injection fixes are all correct. The remaining gap is that
|
| Filename | Overview |
|---|---|
| .github/workflows/pr-comments.yml | Adds top-level permissions block, but the reusable workflow call to RequestNetwork/auto-comments@main remains unpinned; highest-risk file due to pull_request_target + forwarded PAT. |
| .github/workflows/auto-project.yml | Adds permissions: contents: read; reusable workflow call to RequestNetwork/.github@main with PROJECT_TOKEN is still mutable. |
| .github/workflows/reopen-issue-if-prs-open.yml | Adds permissions: contents: read; reusable workflow call to RequestNetwork/.github@main with REOPEN_ISSUES_TOKEN remains pinned to a mutable branch. |
| .github/workflows/auto_assign_pr.yml | Harden Runner added and kentaro-m/auto-assign-action pinned to commit SHA; permissions block was already present. |
| .github/workflows/security-echidna.yml | All actions pinned to commit SHAs, Harden Runner added, issues: write permission added, github-script steps migrated from inline context interpolation to env vars. |
| .github/workflows/security-slither.yml | All actions pinned to commit SHAs including codeql-action/upload-sarif, Harden Runner added, github-script steps migrated to env vars; security-events: write was already present. |
| .github/workflows/tron-smart-contracts.yml | All actions pinned, Harden Runner added to all three jobs, SocketDev firewall applied to install steps, top-level permissions: contents: read added. |
Comments Outside Diff (1)
-
.github/workflows/pr-comments.yml, line 14-19 (link)Reusable workflow still referenced via mutable
@maintagThis workflow uses
pull_request_target(giving it write access to the base repo, even for fork PRs) and passesGH_PAT_AUTO_COMMENTSto an external reusable workflow pinned only at@main. IfRequestNetwork/auto-commentsmain branch is ever compromised, malicious workflow code runs with PR write access and the PAT. The same pattern exists inauto-project.yml(callsRequestNetwork/.github@mainwithPROJECT_TOKEN) andreopen-issue-if-prs-open.yml(same repo withREOPEN_ISSUES_TOKEN). All three need to be pinned to a commit SHA instead of@mainto close the gap this PR aims to address.
Reviews (4): Last reviewed commit: "fix(ci): address PR review comments on s..." | Re-trigger Greptile
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
The nightly failure-alert step calls github.rest.issues.create() to notify the team when Echidna properties fail. Without issues: write the call silently returns a 403 and the alert is never created.
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
The previous CircleCI failures on this PR were caused by the RequestNetwork CircleCI org dropping to the Free plan, which caps Docker resource classes at large. The repo's .circleci/config.yml declares xlarge for build/test jobs (deliberate; see #1703), so every build failed with resource-class-not-in-plan. Org was upgraded to Performance; this empty commit re-triggers the pipeline. No source changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
MantisClone
left a comment
There was a problem hiding this comment.
Approved 👍 pending comment resolution 🚧
Mostly clean; one bug and one scope gap to address before merge.
🔴 Must-fix before merge
github/codeql-action/upload-sarif@5e316336… does not exist in github/codeql-action. Verified three ways: 422 on direct commit lookup, no ref points at it, not in the tag list. The step is if: always() && hashFiles(...) != '' and Slither emits a SARIF every run, so it fails the SARIF upload (and the Slither job) on every contract PR. This is also the exact action #282 called out as highest priority.
Fix: github/codeql-action/upload-sarif@9e0d7b8d25671d64c341c19c0152d693099fb5ba # v4.35.5.
🟡 Scope gap
security-echidna.yml and security-slither.yml both run yarn install --frozen-lockfile unprotected by Socket Firewall. #282 task 4 said Socket on the install step across CI workflows; the PR only wraps the three Tron install steps. Either add sfw to the two security workflows or call out the rationale in the description.
🔵 Nice-to-have before merge
- Bump
# v2/# v4/# v7comments to exact-version (e.g.# v2.11.1,# v4.4.0) so the next reviewer can verify by eye. The floating major tags have already moved past several of the pinned SHAs. - Lift the dedicated
fix(ci): add issues: write…commit subject into the PR description so the permission expansion is documented in the body, not just in git log.
📋 UAT
PR is missing a UAT procedure. Suggested shape:
- After merge, push any contract change to master to trigger Slither, Echidna, and the three Tron jobs.
- Verify each job has Harden Runner as step 1 and (Tron only)
sfw yarn install --frozen-lockfile. - Open https://app.stepsecurity.io and confirm the audit-mode log shows an egress list on each run.
- Confirm the Slither SARIF upload to the Security tab succeeds (this is the failure mode of the must-fix above).
📌 Follow-ups (separate issues, not blockers)
Fresh-eyes pass surfaced things outside this PR's scope. Happy to file these as private-issues if you want:
- Add
Slither Static Analysis,Echidna Property-Based Fuzzing, and the three Tron checks tomaster's required status checks. Currently only CircleCI gates merge. - Enable
sha_pinning_required: trueat the repo or org level so the pinning policy is regression-proof. - Pin
actions/github-script@v6insideRequestNetwork/auto-comments/.github/workflows/pr-auto-comments.yml. That reusable workflow runs underpull_request_targetwithGH_PAT_AUTO_COMMENTS, so the supply-chain threat model leaks one hop in. - Pin the three first-party
@mainreusable workflows (auto-project.yml,pr-comments.yml,reopen-issue-if-prs-open.yml) to SHAs or document accepting the residual risk. - Add
.github/CODEOWNERSrequiring security-team review on.github/workflows/**. - Add
.github/dependabot.yml. The push to this branch surfaced 166 vulns on master (4 critical / 82 high) which is a related theme. - Set
dismiss_stale_reviews: trueon master branch protection.
- Fix invalid codeql-action/upload-sarif SHA (5e316336 -> 9e0d7b8d, v4.35.5)
- Add Socket.dev Firewall to security-slither and security-echidna yarn install steps
- Gate tronbox global install through sfw by swapping step order in tron-smart-contracts
- Use env: + process.env pattern in github-script steps to eliminate ${{ }} script injection
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
Merge activity
|
Implements all GitHub Actions hardening from RequestNetwork/private-issues#282.
Changes
github/codeql-action/upload-sarifwhich runs withsecurity-events: writepermissions: contents: readadded to workflows that lacked it (security workflows already had correct permission blocks)egress-policy: audit)sfw yarn install --frozen-lockfile)Next steps after merge
egress-policy: audit→blockwith the actual allowlist